-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make spaces plugin optional #149044
Make spaces plugin optional #149044
Conversation
Pinging @elastic/kibana-security (Team:Security) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -30,8 +30,7 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban | |||
defaultMessage: 'Banner placement', | |||
}), | |||
description: i18n.translate('xpack.banners.settings.placement.description', { | |||
defaultMessage: | |||
'Display a top banner for this space, above the Elastic header. {subscriptionLink}', | |||
defaultMessage: 'Display a top banner above the Elastic header. {subscriptionLink}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When spaces are enabled, the 'for this space' part of the description is actually very significant, as global banners are configured via a totally different mechanism... But I guess there's not much we can do here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. My original thought was to switch the text based on whether spaces is enabled, but if there is a separate global banner configuration somewhere, this whole section could just be removed when spaces is disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was supposed to approve with my previous review
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @thomheymann |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Response Ops changes LGTM!
## Summary The purpose of this PR is to make the spaces plugin an optional dependency so that it can be disabled in future offerings. In order to achieve this we are reintroducing the following config option to disable spaces: ```yaml xpack.spaces.enabled: false ``` This config option is only available in development mode while we coordinate updating the rest of our plugins. ## Scope In order to keep the scope manageable, only the following plugins have been updated as part of this PR: - `x-pack/plugins/alerting` - `x-pack/plugins/security` - `x-pack/plugins/spaces` The following plugins will need to be updated separately, by working with the corresponding teams: - `x-pack/plugins/cases` - `x-pack/plugins/enterprise_search` - `x-pack/plugins/fleet` - `x-pack/plugins/infra` - `x-pack/plugins/lens` - `x-pack/plugins/ml` - `x-pack/plugins/observability` - `x-pack/plugins/osquery` - `x-pack/plugins/synthetics` ## Screenshots ### Kibana chrome without spaces selector <img width="1073" alt="212935199-dd3bb035-b2f5-4fb6-96e9-5d2093c7992c" src="https://user-images.githubusercontent.com/190132/213432556-18e1c159-1fb1-4808-ad12-47814de575ed.png"> ### Simplified role management screen <img width="1136" alt="Screenshot 2023-01-19 at 11 21 03" src="https://user-images.githubusercontent.com/190132/213432595-0ae33abb-85e7-4044-82a2-6cc44304af0e.png">
closing #155329 <details> <summary>files using space id</summary> ``` apm • server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts: 19 import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; 20: import { addSpaceIdToPath } from '@kbn/spaces-plugin/common'; 21 import { compact } from 'lodash'; 89 isExportable: true, 90: executor: async ({ params, services, spaceId }) => { 91 if (!ml) { 278 279: const viewInAppUrl = addSpaceIdToPath( 280 basePath.publicBaseUrl, 281: spaceId, 282 relativeViewInAppUrl apm • server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts: 19 import { termQuery } from '@kbn/observability-plugin/server'; 20: import { addSpaceIdToPath } from '@kbn/spaces-plugin/common'; 21 import { firstValueFrom } from 'rxjs'; 88 isExportable: true, 89: executor: async ({ params: ruleParams, services, spaceId }) => { 90: console.log('spaceId executor====', spaceId); 91 const predefinedGroupby = [SERVICE_NAME, SERVICE_ENVIRONMENT]; 189 190: const viewInAppUrl = addSpaceIdToPath( 191 basePath.publicBaseUrl, 192: spaceId, 193 relativeViewInAppUrl apm • server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts: 22 import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; 23: import { addSpaceIdToPath } from '@kbn/spaces-plugin/common'; 24 import { firstValueFrom } from 'rxjs'; 104 isExportable: true, 105: executor: async ({ params: ruleParams, services, spaceId }) => { 106 const predefinedGroupby = [ 257 basePath, 258: spaceId, 259 alertUuid 261 262: const viewInAppUrl = addSpaceIdToPath( 263 basePath.publicBaseUrl, 264: spaceId, 265 getAlertUrlTransaction( apm • server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts: 20 import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; 21: import { addSpaceIdToPath } from '@kbn/spaces-plugin/common'; 22 import { firstValueFrom } from 'rxjs'; 98 isExportable: true, 99: executor: async ({ services, spaceId, params: ruleParams }) => { 100: console.log('error rate executor space', spaceId); 101 const predefinedGroupby = [ 251 252: const viewInAppUrl = addSpaceIdToPath( 253 basePath.publicBaseUrl, 254: spaceId, 255 relativeViewInAppUrl apm • server/routes/service_groups/get_service_group_alerts.ts: 24 logger, 25: spaceId, 26 }: { 30 logger: Logger; 31: spaceId?: string; 32 }) { 33: if (!spaceId || serviceGroups.length === 0) { 34 return {}; apm • server/routes/service_groups/route.ts: 189 logger, 190: spaceId: activeSpace?.id ?? DEFAULT_SPACE_ID, 191 }), apm • server/saved_objects/migrations/migrate_legacy_apm_indices_to_space_aware.ts: 79 // Skip default space since it was already updated 80: .filter(({ id: spaceId }) => spaceId !== 'default') 81: .map(({ id: spaceId }) => ({ 82 id: APM_INDEX_SETTINGS_SAVED_OBJECT_ID, 83 type: APM_INDEX_SETTINGS_SAVED_OBJECT_TYPE, 84: initialNamespaces: [spaceId], 85 attributes: savedObjectAttributes, apm • public/components/app/settings/apm_indices/index.tsx: 105 const { data: space } = useFetcher(() => { 106: return services.spaces?.getActiveSpace(); 107 }, [services.spaces]); apm • server/routes/service_groups/route.ts: 174 getApmEventClient(resources), 175: await spacesPluginStart?.spacesService.getActiveSpace(request), 176 ]); ``` </details> ### Notes it's a bit challenging to ensure the other plugins return the default space or have the plugin as optional. How do we want to handle these cases, if there are any? for example, alert returns default space when the plugin is disabled. Relate link PR #149044
Summary
The purpose of this PR is to make the spaces plugin an optional dependency so that it can be disabled in future offerings.
In order to achieve this we are reintroducing the following config option to disable spaces:
This config option is only available in development mode while we coordinate updating the rest of our plugins.
Scope
In order to keep the scope manageable, only the following plugins have been updated as part of this PR:
x-pack/plugins/alerting
x-pack/plugins/security
x-pack/plugins/spaces
The following plugins will need to be updated separately, by working with the corresponding teams:
x-pack/plugins/cases
x-pack/plugins/enterprise_search
x-pack/plugins/fleet
x-pack/plugins/infra
x-pack/plugins/lens
x-pack/plugins/ml
x-pack/plugins/observability
x-pack/plugins/osquery
x-pack/plugins/synthetics
Screenshots
Kibana chrome without spaces selector
Simplified role management screen